fix: onboarding ready condition and missing deps #4153
Conversation
sshanzel
requested review from
rebelchris,
omBratteng,
ilasw,
capJavert,
nensidosari and
AmarTrebinjac
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
1 Skipped Deployment
|
sshanzel
changed the title
| @@ -33,7 +33,7 @@ export const useOnboarding = (): UseOnboarding => { | |||
|
|
|||
| return { | |||
| shouldShowAuthBanner, | |||
| isOnboardingReady: isActionsFetched && isAuthReady, | |||
| isOnboardingReady: isAuthReady && (isActionsFetched || !user), | |||
There was a problem hiding this comment.
What's the issue here I missed it maybe?
There was a problem hiding this comment.
There is an edge case where the loading state is there endlessly. One of the conditions is whether the returned value here is not yet true. The loading never stop since anonymous user would not have actions hence not yet fetched.
There was a problem hiding this comment.
Because of that, the event listeners are not rendered which is needed to process the callback page messages on login/sign up.
| @@ -150,7 +150,6 @@ const seo: NextSeoProps = { | |||
| }; | |||
|
|
|||
| export function OnboardPage(): ReactElement { | |||
| const params = new URLSearchParams(window.location.search); | |||
There was a problem hiding this comment.
Why move it and re-define it multiple places? 🤔
There was a problem hiding this comment.
So that it won't have to be a dependency on the memoized functions. Because if so, we will have to memoize this too since every render would create the object search param.
Also, they are used within those functions and not dependent on anything.
|
Tested the critical flows and worked as intended 🚀 |
Changes
isOnboardingReadyevaluation.Events
Did you introduce any new tracking events?
Experiment
Did you introduce any new experiments?
Manual Testing
Caution
Please make sure existing components are not breaking/affected by this PR
Preview domain
https://revert-revert-condition.preview.app.daily.dev